shell: nushell integration scripts#4630
Conversation
Fixes some path issues. There are still some other things to fix
Pass~[1] is a command line password storage utility. Possible queries are deduced from the file GPG hierarchy under ~/.password-store. [1]: https://www.passwordstore.org/
Not sure why it doesn't work in this context, but if those are not removed, the completion process doesn't substitute the selected value to '**'.
fix typo shell/key-bindinds.nu → shell/key-bindings.nu
There was a problem hiding this comment.
Pull request overview
Adds first-class Nushell integration to fzf, including keybindings, **<TAB> fuzzy completion via Nushell’s external completer mechanism, installation/uninstallation support, and CI/test coverage to prevent regressions.
Changes:
- Add
fzf --nushellflag that prints Nushell integration scripts (keybindings + completion) - Add Nushell install/uninstall handling (autoload
fzf.nu) and update docs - Add Nushell integration tests + Linux CI dependency install for nushell
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
main.go |
Embeds and prints Nushell integration scripts when --nushell is used |
src/options.go |
Adds --nushell option parsing and Options.Nushell flag |
shell/key-bindings.nu |
Implements CTRL-T / CTRL-R / ALT-C bindings for Nushell |
shell/completion.nu |
Implements ** completion via Nushell external completer + command-specific completers |
install |
Adds Nushell to supported shells and generates autoload fzf.nu |
uninstall |
Removes Nushell autoload file during uninstall |
README.md |
Documents Nushell integration setup and limitations |
test/lib/common.rb |
Adds Nushell shell harness + tmux prep behavior |
test/test_shell_integration.rb |
Adds Nushell-specific integration tests (bindings + completion smoke test) |
.github/workflows/linux.yml |
Installs nushell in CI to run Nushell tests |
typos.toml |
Adds Slq to allowed words (for pacman -Slq) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Assuming standard ps output where PID is the second column | ||
| $selected_line | lines | each { $in | from ssv --noheaders | get 0.column1 } | to text |
There was a problem hiding this comment.
This is a false positive. from ssv --noheaders assigns generic column names: column0 = USER, column1 = PID, column2 = PPID, etc. So get 0.column1 correctly extracts the PID. The comment on the line above documents this.
Replace the confusing 'run this in your regular shell' comment with a pure Nushell command using 'save -f'.
The note references fzf 0.48.0 which only introduced --bash, --zsh, and --fish. The --nushell flag will be available in a later release.
Use 'recent versions of fzf' instead of a specific version number as suggested by junegunn. Also add mkdir before save to handle the case where the autoload directory does not exist yet.
f154fa3 to
adbd8e7
Compare
| def test_alt_c_command | ||
| set_var('FZF_ALT_C_COMMAND', 'echo /tmp') | ||
|
|
||
| tmux.prepare | ||
| tmux.send_keys 'cd /', :Enter | ||
|
|
||
| tmux.prepare | ||
| tmux.send_keys :Escape, :c | ||
| tmux.until { |lines| assert_equal 1, lines.match_count } | ||
| tmux.send_keys :Enter | ||
|
|
||
| tmux.prepare | ||
| tmux.send_keys 'pwd', :Enter | ||
| tmux.until { |lines| assert_equal '/tmp', lines[-1] } | ||
| end |
There was a problem hiding this comment.
Same here. Except for :pwd and 'pwd' which should make no difference.
Please review these test cases if redefinitions are necessary.
There was a problem hiding this comment.
Same — removed the redundant redefinition.
There was a problem hiding this comment.
Please review the other tests as well to see if we can minimize code duplication.
There was a problem hiding this comment.
Done — also removed test_ctrl_r (identical to TestShell). The remaining overrides now have comments explaining why they differ:
test_ctrl_t_unicode: uses^echo(external command) since Nushell's builtinechooutputs structured datatest_file_completion: Nushell-specific, tests single-selection only (the external completer replaces the token rather than appending)test_ctrl_r_abort: only tests withfoo— single/double quotes cause issues in Nushell's line editor
There was a problem hiding this comment.
- Nushell-specific, tests single-selection only (the external completer replaces the token rather than appending)
Can you elaborate? We missed the multi-selection bug because of the skipping, so I'd like to keep the test if possible.
There was a problem hiding this comment.
Good point. The test now covers multi-selection (Tab+Tab with 2 files), single selection, and hidden files. The previous override only tested single selection, which is how the multi-selection bug slipped through. Fixed in 4ec77b9.
|
Let me just randomly post issues I noticed in the comments. |
|
Fuzzy completion does not respect export FZF_DEFAULT_OPTS='--tmux 90%,70%' |
|
Multi-selection (tab) in fuzzy completion doesn't work properly. It works for |
Wrap the ls glob in a try/catch so that pass completion returns an empty list instead of a verbose error when ~/.password-store does not exist.
When multiple files are selected with Tab in fzf, join them with spaces so the completer returns a single result instead of separate candidates displayed with ::: separators.
Options from FZF_DEFAULT_OPTS are already parsed and passed as CLI arguments. Without clearing the env var, fzf reads it again, causing --height (CLI) to override --tmux (env) since CLI args take precedence over FZF_DEFAULT_OPTS.
Rewrite __fzf_defaults_nu to return a string (like bash's __fzf_defaults) instead of a list, and pass it via FZF_DEFAULT_OPTS with-env. This ensures --height has a lower index than --tmux from the user's FZF_DEFAULT_OPTS, so --tmux wins when both are present. Also fix a critical bug where the non-existent is-string command silently swallowed all options from FZF_DEFAULT_OPTS.
These tests were identical to the ones inherited from TestShell via include, so the redefinitions were unnecessary.
|
Thanks for testing! Here's a summary of the fixes:
|
Remove test_ctrl_r which was identical to the inherited TestShell version. Add comments explaining why the remaining overrides are necessary: - test_ctrl_t_unicode: uses ^echo (external) instead of echo - test_file_completion: single-selection only (completer replaces token) - test_ctrl_r_abort: skips quote characters that break Nushell
|
Coding agents can miss subtle issues that only surface in hands-on use, which matters a lot for an interactive tool like fzf. Could you share a list of what you manually tested, ideally with screenshots? |
Nushell passes ~/path to the external completer with the tilde unexpanded. Expand it with 'path expand' before passing it to fzf as --walker-root, and restore the tilde prefix in the result.
54c2615 to
b4e1992
Compare
…letion Cover multi-selection (Tab+Tab), single selection, and hidden files. Previously only single selection was tested, which let the multi-selection bug slip through unnoticed.
| - Custom completion extensibility (e.g. `_fzf_complete_COMMAND` in bash/zsh) | ||
| is not available. Custom completions are defined via a `match` statement | ||
| in `completion.nu`. |
There was a problem hiding this comment.
Would it be possible to implement this scheme? A hard-coded match statement feels quite limiting. It could easily turn into a "kitchen sink", with users continually requesting more commands to be added.
For what it's worth, I'm on macOS and don't use tools like pacman or pass, so having them in the default list feels a bit off. I imagine this concern will only grow as more items get added over time.








alt_c,ctrl_randctrl_t**completionCloses: #4122